Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding splitdouble HLSL function #109331

Merged
merged 18 commits into from
Oct 28, 2024
Merged

Adding splitdouble HLSL function #109331

merged 18 commits into from
Oct 28, 2024

Conversation

joaosaffran
Copy link
Contributor

@joaosaffran joaosaffran commented Sep 19, 2024 •
edited
Loading

  • Adding hlsl splitdouble intrinsics
  • Adding DXIL lowering
  • Adding SPIRV lowering
  • Adding test

Fixes: #108901

@joaosaffran joaosaffran force-pushed the intrinsics/splitdouble branch 2 times, most recently from cf135e0 to 8f21239 Compare September 19, 2024 20:50
@joaosaffran joaosaffran force-pushed the intrinsics/splitdouble branch 12 times, most recently from 6d40fb8 to 13a095c Compare September 22, 2024 18:32
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 23, 2024 •
edited
Loading

✅ With the latest revision this PR passed the C/C++ code formatter.

@joaosaffran joaosaffran force-pushed the intrinsics/splitdouble branch 7 times, most recently from 0ca7458 to b7b2831 Compare September 26, 2024 00:26
@joaosaffran joaosaffran marked this pull request as ready for review September 26, 2024 00:33
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:headers Headers provided by Clang, e.g. for intrinsics clang:codegen backend:DirectX labels Sep 26, 2024
@joaosaffran joaosaffran force-pushed the intrinsics/splitdouble branch from 139c244 to 10f4f24 Compare October 25, 2024 05:34
bogner added a commit to bogner/llvm-project that referenced this pull request Oct 25, 2024
@bogner
wip: cherry pick llvm#109331
2d583b7
@joaosaffran joaosaffran force-pushed the intrinsics/splitdouble branch 2 times, most recently from 4f1cbf3 to 37a5a8a Compare October 26, 2024 00:27
bogner
bogner reviewed Oct 26, 2024
View reviewed changes
clang/test/CodeGenHLSL/builtins/splitdouble.hlsl Outdated Show resolved Hide resolved
clang/lib/CodeGen/CGBuiltin.cpp Outdated Show resolved Hide resolved
@joaosaffran joaosaffran force-pushed the intrinsics/splitdouble branch from 24d8571 to b3d58a8 Compare October 26, 2024 06:26
@joaosaffran joaosaffran force-pushed the intrinsics/splitdouble branch from b3d58a8 to 164c6af Compare October 26, 2024 21:14
bogner
bogner approved these changes Oct 27, 2024
View reviewed changes
Copy link
Contributor

@bogner bogner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few last minor nitpicks, but this LGTM!

clang/lib/CodeGen/CGExpr.cpp
Comment on lines +5463 to +5464
std::pair<LValue, LValue>
CodeGenFunction::EmitHLSLOutArgLValues(const HLSLOutArgExpr *E, QualType Ty) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is EmitHLSLOutArgLValues used any other than in CodeGenFunction::EmitHLSLOutArgExpr at this point? I think we can avoid splitting this method in two.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@llvm-beanz, any reason we couldn't merge those methods together?

clang/lib/CodeGen/CGCall.cpp Outdated Show resolved Hide resolved
llvm/test/CodeGen/DirectX/splitdouble.ll Outdated Show resolved Hide resolved
@joaosaffran joaosaffran force-pushed the intrinsics/splitdouble branch from 164c6af to 104479d Compare October 28, 2024 16:32
@joaosaffran joaosaffran merged commit 481bce0 into llvm:main Oct 28, 2024
9 checks passed
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
@joaosaffran @joaosaffran-zz @NoumanAmir657
Adding splitdouble HLSL function (llvm#109331)
6c8898a 
- Adding hlsl `splitdouble` intrinsics
- Adding DXIL lowering
- Adding SPIRV lowering
- Adding test

Fixes: llvm#108901

---------

Co-authored-by: Joao Saffran <jderezende@microsoft.com>
@farzonl farzonl mentioned this pull request Nov 14, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bob80905 bob80905 bob80905 left review comments

@llvm-beanz llvm-beanz llvm-beanz requested changes

@tex3d tex3d tex3d approved these changes

@spall spall spall left review comments

@farzonl farzonl farzonl approved these changes

@bogner bogner bogner approved these changes

Assignees
No one assigned
Labels
backend:DirectX backend:X86 clang:codegen clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:headers Headers provided by Clang, e.g. for intrinsics clang Clang issues not falling into any other category HLSL HLSL Language Support llvm:ir
Projects
HLSL Support
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[HLSL] Implement _split double_ scenario for asuint in HLSL
9 participants
@joaosaffran @llvmbot @bogner @spall @farzonl @tex3d @llvm-beanz @bob80905 @joaosaffran-zz